Skip to content

Fix race condition in Otsu's method #3970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 17, 2025
Merged

Fix race condition in Otsu's method #3970

merged 5 commits into from
Jul 17, 2025

Conversation

troelsy
Copy link
Contributor

@troelsy troelsy commented Jul 14, 2025

I found that in some cases, Otsu's method can have two or more equal thresholds. In case of the CUDA implementation, this would create a race-condition where the last thread to write the output gets to choose which Otsu threshold to use. The resulting image from cv::cuda::threshold would be the same, but the returned threshold would vary for each run.

I solve this by first doing a poll to check if there are multiple matches with __syncthreads_count. If not, we can return the threshold as before. If there are more than one, the kernel does a minimum reduction to find the lowest threshold that matches the otsu score. It doesn't matter which threshold you choose as long as it is consistent, so I choose the smallest one.

I'm unsure when __syncthreads_count as introduced, but it is compatible with CC≥2.0. CUDA Toolkit's archive only goes back to version 8.0, but it was documented back then (https://docs.nvidia.com/cuda/archive)

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@cudawarped
Copy link
Contributor

@troelsy Can you provide a test case which fails without this fix?

@cudawarped
Copy link
Contributor

@troelsy It might be simpler to alter the reduction to account for this, see 7a3c94a.

I would check which approach is more efficient for larger images.

@troelsy
Copy link
Contributor Author

troelsy commented Jul 15, 2025

I should be able to generate an image with multiple matches, but I'll have to try some things out. In my own test I generated the histogram without an image. An image with 50% white and 50% black should do it. I'll make a test

The image size is not important in this part of the code as it only runs on the histogram. I did also consider your solution and it may be more suiting for OpenCV than us. It is a gamble on how likely it is to happen and I haven't been able to trigger the minimum reduction on our footage. It is probably more likely on computer generated images

@cudawarped
Copy link
Contributor

I did also consider your solution and it may be more suiting for OpenCV than us.

I did a quick bit of benchmarking, on an RTX 3070 TI your approach is quicker regardless of whether there is more than one unique solution (matches > 1).

Copy link
Contributor

@cudawarped cudawarped left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@asmorkalov asmorkalov merged commit 8402454 into opencv:4.x Jul 17, 2025
9 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants